-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: make kurtosis service logs faster #2525
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tedim52
commented
Aug 9, 2024
tedim52
force-pushed
the
tedi/logspeedup
branch
from
August 10, 2024 14:02
5b7a352
to
2c1f0bf
Compare
h4ck3rk3y
approved these changes
Aug 12, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 16, 2024
🤖 I have created a release *beep* *boop* --- ## [1.1.0](1.0.0...1.1.0) (2024-08-16) ### Features * [log retention improvements pt. 1] introduce file layout interface ([#2534](#2534)) ([a494278](a494278)) * make kurtosis service logs faster ([#2525](#2525)) ([d6b246a](d6b246a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: kurtosisbot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Users were experiencing
kurtosis service logs
taking a long time. After running tests, I discovered that a majority of execution time during log processing was spent in the following lines:Prior to this change, we were sending logs one at a time on an unbuffered channel - unbuffered channels block until the receiving goroutine reads the value. This was causing a lot of time being wasted waiting to send log lines across the channel.
This change implements a
LogLineSender
that:With this change, the time to read 20 minutes of
cl-lighthouse-geth
logs with log level set to debug went from1min53sec
to30.055
seconds. The time to read 2 hours 10 minutes worth ofcl-lighthouse
debug logs (around 3.4 gb of logs) went from15min1sec
to3min31
sec. (As a benchmark,cat logs.json
on3.4 gb
of logs takes around2min
- on my machine - so much closer) This can likely be improved further by tuning the buffer size and batch amount.Is this change user facing?
YES
References:
https://discord.com/channels/783719264308953108/1267837033032974467/1267842228072611881